Skip to content

Adding Cmdlet New-AzApplicationGatewayRewriteRuleUrlConfiguration to support url rewrite capability for V2 application gateways#10894

Merged
anton-evseev merged 6 commits intoAzure:network-decemberfrom
abjai:urlrewrite
Jan 21, 2020
Merged

Adding Cmdlet New-AzApplicationGatewayRewriteRuleUrlConfiguration to support url rewrite capability for V2 application gateways#10894
anton-evseev merged 6 commits intoAzure:network-decemberfrom
abjai:urlrewrite

Conversation

@abjai
Copy link
Contributor

@abjai abjai commented Jan 14, 2020

Description

This should be released as a part of Powershell January Release

Adding Support for UrlConfiguration under RewriteRuleActionset for Application Gateway Url Rewrite Feature

Checklist

  • I have read the Submitting Changes section of CONTRIBUTING.md
  • The title of the PR is clear and informative
  • The appropriate ChangeLog.md file(s) has been updated:
    • For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
    • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
  • The PR does not introduce breaking changes
  • If applicable, the changes made in the PR have proper test coverage
  • For public API changes to cmdlets:
    • a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
    • the markdown help files have been regenerated using the commands listed here

Powershell design review link: Azure/azure-powershell-cmdlet-review-pr#339
Swagger PR: Azure/azure-rest-api-specs#7905

abjai
abjai commented Jan 14, 2020
View reviewed changes
@abjai abjai requested a review from anton-evseev January 14, 2020 14:34
isra-fel
isra-fel requested changes Jan 15, 2020
View reviewed changes
Copy link
Member

@isra-fel isra-fel left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @abjai, please see my inline comments. Thanks!
Oh and please also fix the merge conflict.

@isra-fel
Copy link
Member

isra-fel commented Jan 15, 2020

/azp run

@azure-pipelines
Copy link
Contributor

azure-pipelines bot commented Jan 15, 2020

Pull request contains merge conflicts.

@isra-fel isra-fel assigned abjai and unassigned isra-fel Jan 15, 2020
@isra-fel isra-fel assigned isra-fel and unassigned abjai Jan 16, 2020
@abjai
Copy link
Contributor Author

abjai commented Jan 16, 2020

Hi @abjai, please see my inline comments. Thanks!
Oh and please also fix the merge conflict.

@isra-fel I have addressed your comments. I have added the local feed so that tests can run. Once swagger SDK got published and updated in december-release branch, i will pull those changes and resolve the conflicts.

Please note that we are targetting these changes to be released as a part of January end release.

@isra-fel
Copy link
Member

isra-fel commented Jan 19, 2020 •
edited
Loading

Hi @abjai
I'm good with the change. However the CI is blocked by the merge conflict.

@abjai
Copy link
Contributor Author

abjai commented Jan 20, 2020

Hi @abjai
I'm good with the change. However the CI is blocked by the merge conflict.

@isra-fel : I have removed the conflict changes however i could see that SDK is still not updated in Network.csproj so tests might fail.

@number213 : Can you please update the network.csproj with latest SDK so that tests start passing?

Please note that these changes need to be part of Januray 20 release

@isra-fel
Copy link
Member

isra-fel commented Jan 20, 2020

So, your test project still relies on 21.0.00preview SDK.

@isra-fel isra-fel assigned abjai and unassigned isra-fel Jan 20, 2020
@abjai
Copy link
Contributor Author

abjai commented Jan 20, 2020

So, your test project still relies on 21.0.00preview SDK.

@isra-fel : Yes it is still dependent on the updated SDK published as a part of January 17 release. I don't know the exact version published as a part of SDK release. Adding @number213 to help in updating the SDK version for Network.csproj

@abjai
Copy link
Contributor Author

abjai commented Jan 20, 2020

So, your test project still relies on 21.0.00preview SDK.

@isra-fel : Yes it is still dependent on the updated SDK published as a part of January 17 release. I don't know the exact version published as a part of SDK release. Adding @number213 to help in updating the SDK version for Network.csproj

@isra-fel @number213 has raised a PR to update SDK #10928. Once it is merged, my changes should be able to pass the tests

@abjai abjai changed the base branch from master to network-december January 20, 2020 11:15
@abjai abjai changed the base branch from network-december to master January 20, 2020 11:23
@abjai abjai changed the base branch from master to network-december January 20, 2020 11:37
@abjai
Copy link
Contributor Author

abjai commented Jan 20, 2020

So, your test project still relies on 21.0.00preview SDK.

@isra-fel : Yes it is still dependent on the updated SDK published as a part of January 17 release. I don't know the exact version published as a part of SDK release. Adding @number213 to help in updating the SDK version for Network.csproj

@isra-fel @number213 has raised a PR to update SDK #10928. Once it is merged, my changes should be able to pass the tests

@isra-fel : updated the PR with latest sdk.

isra-fel
isra-fel approved these changes Jan 21, 2020
View reviewed changes
Copy link
Member

@isra-fel isra-fel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@isra-fel isra-fel assigned isra-fel and unassigned abjai Jan 21, 2020
@isra-fel
Copy link
Member

isra-fel commented Jan 21, 2020

/azp run azure-powershell - powershell-core

@azure-pipelines
Copy link
Contributor

azure-pipelines bot commented Jan 21, 2020

Azure Pipelines successfully started running 1 pipeline(s).

@isra-fel isra-fel mentioned this pull request Jan 21, 2020
Open
@isra-fel
Copy link
Member

isra-fel commented Jan 21, 2020

/azp run azure-powershell - powershell-core

@azure-pipelines
Copy link
Contributor

azure-pipelines bot commented Jan 21, 2020

Azure Pipelines successfully started running 1 pipeline(s).

@anton-evseev
Copy link
Contributor

anton-evseev commented Jan 21, 2020

Need to work on PR for merging network into master. Since @isra-fel approved and added "waiting for CI" I'll go ahead and merge this PR

@anton-evseev anton-evseev merged commit 295cdcd into Azure:network-december Jan 21, 2020
dingmeng-xue pushed a commit to dingmeng-xue/azure-powershell that referenced this pull request Mar 29, 2020
@anton-evseev
Merge pull request Azure#10894 from abjai/urlrewrite
c47a441 
Adding Cmdlet New-AzApplicationGatewayRewriteRuleUrlConfiguration to support url rewrite capability for V2 application gateways
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@isra-fel isra-fel isra-fel approved these changes

+1 more reviewer

@anton-evseev anton-evseev anton-evseev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@isra-fel isra-fel

Labels

Waiting for CI :shipit:

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@abjai @isra-fel @anton-evseev

Comments